npe guard for get host info on vmware#11054
Merged
DaanHoogland merged 4 commits intoapache:4.19from Jul 17, 2025
Merged
Conversation
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
| if (aboutInfo == null) { | ||
| String msg = "no type info about host known" | ||
| s_logger.error(msg) | ||
| throw new Exception(msg); |
Member
There was a problem hiding this comment.
can it be defaulted to VmwareHostType.ESXi ?
Contributor
Author
There was a problem hiding this comment.
what does it mean if the host does not have this info? Is it in a valid state then?
Member
There was a problem hiding this comment.
I never face this issue before, but I know some users have faced the issue.
If we throw an Exception with message, instead of NPE, I think it does not help, as user will still get an exception.
Contributor
Author
There was a problem hiding this comment.
yes, but at least it will be clear they have an ill-configured host.
Member
There was a problem hiding this comment.
to be honest, I do not know what is misconfigured and any other issues caused by the misconfiguration.
I think we could consider the host as an ESXi host if the about info is not found.
anyone uses ESX host or other type of host ?
Contributor
Author
There was a problem hiding this comment.
ok, if y’all insist ;)
Member
There was a problem hiding this comment.
let's get more opinions
@nvazquez @harikrishna-patnala @sureshanaparti @shwstppr @rohityadavcloud
what's your opinion on it ?
Contributor
Author
There was a problem hiding this comment.
what I meant @weizhouapache , is your argument makes sense, I yield. Please see my changed code fragment at #11054 (comment).
Member
There was a problem hiding this comment.
agreed, assuming ESXi seems to be a good option.
DaanHoogland
commented
Jun 19, 2025
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostMO.java
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #11054 +/- ##
=========================================
Coverage 15.18% 15.18%
- Complexity 11364 11365 +1
=========================================
Files 5415 5415
Lines 475858 475862 +4
Branches 58092 58093 +1
=========================================
+ Hits 72251 72252 +1
- Misses 395521 395525 +4
+ Partials 8086 8085 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
weizhouapache
approved these changes
Jun 20, 2025
Member
weizhouapache
left a comment
weizhouapache
left a comment
There was a problem hiding this comment.
code lgtm
thanks @DaanHoogland for the update
nvazquez
approved these changes
Jun 20, 2025
DaanHoogland
force-pushed
the
ghi10930-NPEGuard
branch
from
f66ce65 to
d3857c8
Compare
DaanHoogland
commented
Jun 23, 2025
vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostMO.java
Outdated
Show resolved
Hide resolved
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13884 |
Contributor
Author
|
@blueorangutan test ol8 vmware-80u3 |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + vmware-80u3) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-13662)
|
|
[SF] Trillian test result (tid-13700)
|
Contributor
Author
|
I think vmware needs attention, but none of these are related to the change afaict. |
DaanHoogland
force-pushed
the
ghi10930-NPEGuard
branch
from
0524523 to
0a92c41
Compare
Contributor
Author
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 14086 |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14094 |
Contributor
Author
|
@blueorangutan test ol8 vmware-70u3 |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + vmware-70u3) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-13724)
|
|
[SF] Trillian test result (tid-13699)
|
borisstoyanov
approved these changes
Jul 11, 2025
Contributor
borisstoyanov
left a comment
borisstoyanov
left a comment
There was a problem hiding this comment.
LGTM, based on code review
Contributor
Author
|
this seems fine with vmware 70u3, just out of insanity trying 80u3, but i think we can merge and have to give the rest attention in other efforts. |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + vmware-80u3) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-13767) |
Member
|
I think this can be merged right! @DaanHoogland The failures does not seems to be related this change |
Contributor
these tests are ok, I think, this is good to go @DaanHoogland |
|
[SF] Trillian test result (tid-13768)
|
harikrishna-patnala
pushed a commit
to shapeblue/cloudstack
that referenced
this pull request
Jul 24, 2025
Co-authored-by: Daan Hoogland <dahn@apache.org>
dhslove
pushed a commit
to ablecloud-team/ablestack-cloud
that referenced
this pull request
Aug 6, 2025
Co-authored-by: Daan Hoogland <dahn@apache.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR
Fixes: #10930
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?